-
Notifications
You must be signed in to change notification settings - Fork 5
fix for registering nested message types #23
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @thiagodpf !
First of all, I'd like to thank you for raising the issue and working on a fix 🙇 Great job!
Regarding the PR, it looks good 👍
However, the one possible improvement could be implementing the stack here instead of using the github.com/golang-collections/collections package (the go implementation is pretty straightforward). The rationale is to reduce the number of libraries we import, improving maintainability.
How does that sound?
Hi @olegbespalov !! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! 🚀
Thank you for your contribution! 🙇
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the work on this 🙇
@thiagodpf just a tiny update on the next steps 👍 We're going to pack some more changes into the xk6-grpc "release" and once it is done, bump the new version in the primary k6's repo. Once that is done, the k6's experimental module will get the fixes. In parallel, we're going to backport the changes to the legacy Thanks again! 🚀 |
@olegbespalov thanks for accepting my PR! |
Done! ✌️😃 Again, thanks for the support and help! |
@mstoykov Related to this issue.